pallet scheduler: fix weight and add safety checks#7785
Conversation
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
/cmd bench --runtime westend asset-hub-westend |
|
Command "bench --runtime westend asset-hub-westend" has started 🚀 See logs here |
- **Add test to collectives pallet** - **Manually edit scheduler weight** ## Investigation The Rust compiler changed how the `stringify!` macro formats paths on [Aug 12th '24](rust-lang/rust#128992 (comment)). I assume that this broke the V1 benchmarking [here](https://github.com/paritytech/polkadot-sdk/blob/7ecf3f757a5d6f622309cea7f788e8a547a5dce8/substrate/frame/benchmarking/src/v1.rs#L1011). We had updated the scheduler pallet to V2 syntax (which is [not affected](https://github.com/paritytech/polkadot-sdk/blob/df99fb9431a579589c1c87832222c9551b5c4f7c/substrate/frame/support/procedural/src/benchmark.rs#L565)) on [Nov 20th '24](paritytech/polkadot-sdk#6292). We did not back-port this since there was no apparent reason for it. The is why it seemingly fixed it self on SDK master but not in the runtimes repo (since that is using V1 scheduler benchmarking). Another indication for this is the fact that it did work on the [whitelist pallet](https://github.com/polkadot-fellows/runtimes/blob/6b85bf6adb427942976648e6d235e0169dfced16/relay/polkadot/src/weights/pallet_whitelist.rs#L85), which is on V2 much longer but used the same [custom pov mode](https://github.com/paritytech/polkadot-sdk/blob/b76e91acc953e682b3ddcfc45ecacaaf26c694a1/substrate/frame/whitelist/src/benchmarking.rs#L68). Adding some sanity checks to prevent this in the future here paritytech/polkadot-sdk#7785 - [ ] Does not require a CHANGELOG entry --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: GitHub Action <action@github.com>
|
Command "bench --runtime westend asset-hub-westend" has finished ✅ See logs here DetailsSubweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…sset-hub-westend'" This reverts commit bb19d78.
|
/cmd prdoc --bump minor --audience runtime_dev |
bkchr
left a comment
There was a problem hiding this comment.
One nitpick, otherwise looks good.
|
This is still a problem. Moonbeam is using the latest changes from branch stable2412 and the issue is not solved. |
|
We should port the benchmark v2 migration to the stable2412 branch at least, otherwise this will for sure impact other projects |
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
Successfully created backport PR for |
|
Git push to origin failed for stable2407 with exitcode 1 |
Changes: - Add runtime integrity test for scheduler pallet to ensure that lookups use sensible weights - Check all passed storage names in the omni bencher to be known by FRAME metadata - Trim storage names in omni bencher to fix V1 bench syntax bug - Fix V1 bench syntax storage name sanitization for specific Rust versions I re-ran the benchmarks with the omni-bencher modifications and it did not change the [proof size](https://weights.tasty.limo/compare?repo=polkadot-sdk&threshold=1&path_pattern=substrate%2Fframe%2F**%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs&method=asymptotic&ignore_errors=true&unit=proof&old=cc0142510b81dcf1c1a22f7dc164c453c25287e6&new=bb19d78821eaeaf2262f6a23ee45f83dd4f94d29). I reverted [the commit](bb19d78) afterwards to reduce the noise for reviewers. --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit 058b4f5)
|
Successfully created backport PR for |
|
Git push to origin failed for stable2409 with exitcode 1 |
Changes: - Add runtime integrity test for scheduler pallet to ensure that lookups use sensible weights - Check all passed storage names in the omni bencher to be known by FRAME metadata - Trim storage names in omni bencher to fix V1 bench syntax bug - Fix V1 bench syntax storage name sanitization for specific Rust versions I re-ran the benchmarks with the omni-bencher modifications and it did not change the [proof size](https://weights.tasty.limo/compare?repo=polkadot-sdk&threshold=1&path_pattern=substrate%2Fframe%2F**%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs&method=asymptotic&ignore_errors=true&unit=proof&old=cc0142510b81dcf1c1a22f7dc164c453c25287e6&new=bb19d78821eaeaf2262f6a23ee45f83dd4f94d29). I reverted [the commit](bb19d78) afterwards to reduce the noise for reviewers. --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> (cherry picked from commit 058b4f5)
|
Successfully created backport PR for |
|
Git push to origin failed for stable2412 with exitcode 1 |
|
Git push to origin failed for stable2503 with exitcode 1 |
Backport #7785 into `stable2503` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Backport #7785 into `stable2407` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Backport #7785 into `stable2409` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Backport paritytech#7785 into `stable2503` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Update scheduler weights for #8726 This was originally a backport of #7785 but is now re-purposed. --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Backport paritytech#7785 into `stable2503` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Backport paritytech#7785 into `stable2503` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Backport paritytech#7785 into `stable2503` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor_P <egor@parity.io>
Changes: - Add runtime integrity test for scheduler pallet to ensure that lookups use sensible weights - Check all passed storage names in the omni bencher to be known by FRAME metadata - Trim storage names in omni bencher to fix V1 bench syntax bug - Fix V1 bench syntax storage name sanitization for specific Rust versions I re-ran the benchmarks with the omni-bencher modifications and it did not change the [proof size](https://weights.tasty.limo/compare?repo=polkadot-sdk&threshold=1&path_pattern=substrate%2Fframe%2F**%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs&method=asymptotic&ignore_errors=true&unit=proof&old=cc0142510b81dcf1c1a22f7dc164c453c25287e6&new=bb19d78821eaeaf2262f6a23ee45f83dd4f94d29). I reverted [the commit](bb19d78) afterwards to reduce the noise for reviewers. --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Changes:
I re-ran the benchmarks with the omni-bencher modifications and it did not change the proof size. I reverted the commit afterwards to reduce the noise for reviewers.